Generic controller-manager config #1960
Conversation
ghost
added
the
jackfrancis
force-pushed
the
generic-controller-manager
branch
from
c2717ce
to
e0e53e2
Compare
jackfrancis
changed the title
|
@CecileRobertMichon FYI this PR also separates out kubelet config default handling into a separate file. |
jackfrancis
force-pushed
the
generic-controller-manager
branch
2 times, most recently
from
2e471d5
to
003487f
Compare
jackfrancis
force-pushed
the
generic-controller-manager
branch
from
003487f
to
9a00460
Compare
| @@ -862,6 +859,28 @@ func (t *TemplateGenerator) getTemplateFuncMap(cs *api.ContainerService) templat | |||
| } | |||
| return buf.String() | |||
| }, | |||
| "GetControllerManagerConfigKeyVals": func(kc *api.KubernetesConfig) string { | |||
There was a problem hiding this comment.
I do not see kc is used here. It is ok if you plan to use it in the future and want to place it here for now.
There was a problem hiding this comment.
Good catch, will address in a follow-up commit.
| "github.com/Azure/acs-engine/pkg/api" | ||
| ) | ||
|
|
||
| func setControllerManagerConfig(cs *api.ContainerService) { |
There was a problem hiding this comment.
Since o.KubernetesConfig.ControllerManagerConfig is now exposed to user, it looks we allow user to define other configs beyond the list below (default + static + rbac) but in https://kubernetes.io/docs/reference/generated/kube-controller-manager/ . Do we want to support this?
There was a problem hiding this comment.
Yes, that's what this statement in the doc update supports:
We consider
kubeletConfigandcontrollerManagerConfigto be generic conveniences that add power/flexibility to cluster deployments. Their usage comes with no operational guarantees! They are manual tuning features that enable low-level configuration of a kubernetes cluster.
pkg/api/vlabs/validate.go
Outdated
| if a.CtrlMgrNodeMonitorGracePeriod == "" { | ||
| return fmt.Errorf("--node-status-update-frequency was set to '%s' but OrchestratorProfile.KubernetesConfig.CtrlMgrNodeMonitorGracePeriod was not set", val) | ||
| if a.ControllerManagerConfig != nil { | ||
| if a.ControllerManagerConfig["--node-monitor-grace-period"] == "" { |
There was a problem hiding this comment.
Do we still need this check and some other checks below since we provide a default value?
There was a problem hiding this comment.
Yep, you're right, addressing in a follow-up commit.
- actually using passed-in *api.KubernetesConfig reference in GetControllerManagerConfigKeyVals() - removing unnecessary validations for both controller manager and kubelet
What this PR does / why we need it: exposes a generic key/val interface for configuring controller-manager, using the configuration keys documented here:
https://kubernetes.io/docs/reference/generated/kube-controller-manager/
API model usage patterns will look like this: